Skip to content

fix(test): correct the platform-tool allowlist — search_knowledge is real - #564

Merged
yinlianghui merged 1 commit into
mainfrom
claude/copilots-undefined-tools-1n2wn1
Jul 31, 2026
Merged

fix(test): correct the platform-tool allowlist — search_knowledge is real#564
yinlianghui merged 1 commit into
mainfrom
claude/copilots-undefined-tools-1n2wn1

Conversation

@yinlianghui

@yinlianghui yinlianghui commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Description

Corrects a bug I introduced in #557, plus the wrong premise it was built on.

The PLATFORM_TOOLS allowlist in test/skills-integrity.test.ts guessed the platform tool set from what the @objectstack/mcp@16.1.0 bridge happens to register. It was wrong in both directions:

  • It omitted search_knowledge, so the guard would have failed a legitimate reference. The tool is real: it is in PLATFORM_PROVIDED_TOOL_NAMES, and 16.1.0 documented it all along at spec/src/ai/knowledge-source.zod.ts:114 — "Whether search_knowledge may expose this source to AI agents." One grep for that name would have caught it. The same absent-from-node_modules inference was applied to visualize_data and came out the other way, which should have been the tell.
  • It reasoned about deliberately excluding create_record / update_record / delete_record. Those are MCP-bridge tools and are not in the platform registry at all — there was nothing to exclude.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • CI/CD update

Related Issues

Related to #493, #512, #557, #561
Upstream: objectstack-ai/objectstack#3820

Changes Made

  • test/skills-integrity.test.ts — the allowlist is now transcribed verbatim from PLATFORM_PROVIDED_TOOL_NAMES in @objectstack/spec@17.0.0-rc.0 (all 30 entries, diffed equal to the upstream set), carrying instructions to delete the literal and import { PLATFORM_PROVIDED_TOOL_NAMES } from '@objectstack/spec/system' on the 17.0 upgrade. A hand-copied registry is precisely the drift risk this file exists to catch one level up; it is a stopgap for 16.1.0, which exports no such set.
  • src/skills/customer-360.skill.ts — docstring corrected. The skill still does not reference search_knowledge, but for an accurate and narrower reason (below). The instruction line now says the model has no such tool in this skill, rather than that none exists — the old wording was false to the model too.
  • .changeset/skills-undefined-tool-references.md (not yet consumed by a release) — the "nothing defines it" claim removed, so it does not ship into the changelog.
  • docs/developers/code_examples.md — "Platform data tools" was an incomplete list presented as complete; now points at the registry as the source of truth and names the 17.0 export.

Why the skill still leaves the tool out

search_knowledge retrieves over a declared knowledge source. AIKnowledgeSchema mounts only on AgentSchema.knowledge — the indexes: [...] block — and #512 deleted the agents. A skills-only app has nowhere to declare a source, so the tool would resolve and return nothing. That is the same lie one layer down, so crm_knowledge_article via query_records remains the right call. Worth revisiting if a stack-level knowledge source becomes declarable.

Testing

  • Unit tests pass (npx vitest run) — 13 files, 168 passed / 2 skipped
  • Linting passes; build succeeds; objectstack validate holds at its 2 pre-existing campaign_enrollment warnings
  • Manual testing completed — see the differential check below
  • New tests added (if applicable) — no new cases; the existing guard is corrected and cross-validated

Differential check against the upstream rule

@objectstack/lint@17.0.0-rc.0 ships ai-skill-tool-unresolved (the rule objectstack#3820 tracks). Installed the rc into a scratch tree and ran both it and this repo's guard over the same probes:

reference upstream rule this guard
search_knowledge accept accept
query_data accept accept
todo_write accept accept
action_convert_lead accept accept
action_escalate_case (not ai.exposed) reject reject
search_knowledgebase (typo) reject reject
triage_case (fictional) reject reject

Identical verdicts on all seven. The upstream rule also reports 0 findings against the pre-#557 state — independent confirmation that removing search_knowledge as "undefined" was wrong.

Checklist

  • I have added a changeset (.changeset/skills-platform-tool-allowlist-correction.md, patch)
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional Notes

One divergence found while cross-checking, worth carrying upstream if anyone picks up objectstack-ai/objectstack#3820: the upstream rule's collectToolUniverse counts stack.tools metadata records as resolving, but ToolSchema in 17.0 still describes itself as a [READ-ONLY PROJECTION — not an execution entry point] with no executor. A metadata-authored tool would satisfy the linter and still never run. This guard's does not lean on AI tool metadata case is stricter on that point and is kept.

The 17.0 upgrade itself is deliberately not in this PR — 17.0.0-rc.0 is a release candidate and bumping it touches the whole dependency tree. This change is dependency-neutral.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y9STuduWbrAwcgviziaV4e

Generated by Claude Code

@claude
fix(test): correct the platform-tool allowlist — search_knowledge is …
dbdf51f 
…real

The allowlist added in #557 guessed the platform tool set from what the
`@objectstack/mcp@16.1.0` bridge happens to register, and got it wrong in
both directions:

- It omitted `search_knowledge`, so the guard would have FAILED a
  legitimate reference. The tool is real — it is in
  `PLATFORM_PROVIDED_TOOL_NAMES`, and 16.1.0 documented it all along at
  `spec/src/ai/knowledge-source.zod.ts:114` ("Whether `search_knowledge`
  may expose this source to AI agents"). One grep for that name would
  have caught it; the same absent-from-node_modules inference was
  applied to `visualize_data` and came out the other way.
- It reasoned about deliberately excluding `create_record` /
  `update_record` / `delete_record`. Those are MCP-bridge tools and are
  not in the platform registry at all, so there was nothing to exclude.

The list is now transcribed verbatim from `PLATFORM_PROVIDED_TOOL_NAMES`
in `@objectstack/spec@17.0.0-rc.0` — all 30 entries, diffed equal to the
upstream set — carrying instructions to delete the literal and import it
on the 17.0 upgrade. A hand-copied registry is the drift risk this file
exists to catch, one level up; it is a stopgap for 16.1.0, which exposes
no such export.

Cross-checked against upstream's own `ai-skill-tool-unresolved` rule,
which ships in `@objectstack/lint@17.0.0-rc.0` (objectstack#3820). Both
give identical verdicts on seven probes: `search_knowledge`,
`query_data`, `todo_write` and `action_convert_lead` accepted;
`action_escalate_case` (not `ai.exposed`), `search_knowledgebase` and
`triage_case` rejected.

Also corrects the "nothing defines it" claim in three places — the
`customer_360` docstring, its changeset, and `code_examples.md`. The
skill still does not reference the tool, for a narrower and accurate
reason: retrieval needs a declared knowledge source, `AIKnowledgeSchema`
mounts only on `AgentSchema.knowledge`, and #512 deleted the agents — so
a skills-only app has nowhere to declare one and the tool would resolve
and return nothing. The skill's instruction line now says the model has
no such tool *in this skill*, rather than that none exists.

168 tests pass, typecheck and build clean, validate holds at its 2
pre-existing warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9STuduWbrAwcgviziaV4e
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Jul 31, 2026 3:32am

Request Review

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jul 31, 2026
@yinlianghui
yinlianghui marked this pull request as ready for review July 31, 2026 03:41
@yinlianghui
yinlianghui merged commit 3260107 into main Jul 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yinlianghui @claude